Skip to content

docs: make the README a doctest and correct the public-API docs (#99) - #134

Merged
Xof merged 1 commit into
docs/98-architecture-driftfrom
docs/99-public-api-docs
Jul 31, 2026
Merged

docs: make the README a doctest and correct the public-API docs (#99)#134
Xof merged 1 commit into
docs/98-architecture-driftfrom
docs/99-public-api-docs

Conversation

@Xof

@Xof Xof commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

Closes #99 (4 findings, DESIGN/api-design+docs).

Stacked on #133#132#131#130#129main.

Two of the four findings were already fixed earlier in this stack:
PUBLIC-API-2 (Poisoned listed as fatal while is_fatal() returns false
for it) landed with #133, and PUBLIC-API-4's encrypted-database
under-report landed with #129 as the stride fix. What follows is the remainder.

PUBLIC-API-3 — every README example was a compile error

Against the current public API:

  • use chisel::defrag::DefragOptions; — the module is pub(crate); the type is re-exported at the crate root
  • DefragOptions { sparse_threshold: 0.25, max_pages: 0 } — the field is max_values, and the struct is #[non_exhaustive] so a downstream crate cannot use a struct literal at all
  • let options = Options { … }; — same #[non_exhaustive] problem, which the very next paragraph then explains
  • db.allocate_tagged(b"row-a", 42), assert_eq!(db.tag(a)?, 42), db.handles_with_tag(42) — these take Tag and return Option<Tag> / Handle since I120/I126

Nothing caught any of it, because the README was not wired into the build.

The fix is the wiring, not just the snippets

Correcting the examples alone would leave them to rot on the next API change.
The README is now included via #![doc = include_str!("../README.md")], so all
ten fences are doctests and cargo test fails the next time the docs fall
behind the API. Fences that would touch the filesystem, or need a live failure
to illustrate, are no_run — still compiled, just not executed; the rest run
against an in-memory database set up in hidden (#-prefixed) lines.

test src/lib.rs - (line 171) ... ok
test src/lib.rs - (line 193) ... ok
...
test result: ok. 10 passed; 0 failed

Verified load-bearing rather than decorative — reverting one example to its old
form:

error[E0308]: mismatched types
   |            ---------------           ^^ expected `Tag`, found integer
test result: FAILED. 9 passed; 1 failed

The surrounding prose is corrected to match: tags are non-zero and untagged
means tag() returns None (the "tag 0 means untagged" model predates
I126), and the API table names Handle/Tag rather than u64/u32.

PUBLIC-API-4 (remainder) — both Stats fields were described wrongly, in opposite directions

total_pages claimed to match Superblock.total_pages but is filled from
file_page_count() — the physical count. Worth naming explicitly, because
the two diverge exactly when a crash leaves orphan pages in the tail, and the
superblock figure is the authoritative one for what the database contains.

file_size_bytes claimed it "may exceed total_pages * PAGE_SIZE when a
previous crash left orphan pages" — but it is computed from that same count, so
it can never exceed itself. The impossible clause is gone.

PUBLIC-API-5 — # Errors omitted InvalidRootName on two methods

get_root_name documented "Only on poisoning — an unbound name returns
Ok(None)", and clear_root_name documented only NoActiveTransaction. Both
delegate to inner functions that call encode_root_name(name)?, so "", a
25-byte name, or a name containing NUL is an InvalidRootName error — not a
miss.

A caller trusting "only on poisoning" treats any Err from a lookup as a
fatal drop-and-reopen condition and tears down a healthy handle over a too-long
name. #![warn(clippy::missing_errors_doc)] guarantees the section exists
but not that it is complete, which is what makes an omission like this easy
to trust.

New testroot_name_validation_applies_to_the_read_and_clear_paths_too
pins the corrected contract on both methods, including the half that is easy to
over-correct: a valid-but-unbound name is still Ok(None) / Ok(()).

Verification

cargo test all green (including the 10 new doctests),
cargo clippy --workspace --all-targets -- -D warnings clean,
cargo fmt --check clean.

Two of this issue's four findings were already fixed earlier in the
stack — PUBLIC-API-2 (Poisoned vs is_fatal) landed with #98, and
PUBLIC-API-4's encrypted-database under-report landed with #94 as the
stride fix. What remains:

Every Rust snippet in the README was a compile error against the
current public API: `use chisel::defrag::DefragOptions` (the module went
pub(crate)), struct literals against `#[non_exhaustive]` `DefragOptions`
and `Options`, a `max_pages` field that is called `max_values`, and bare
integers where the post-I120/I126 `Tag` and `Handle` newtypes are
required. The README is the entry point for every downstream user and
nothing caught any of it, because the README was not wired into the
build at all.

Rather than only correcting the snippets — which would rot again on the
next API change — the README is now included via
`#![doc = include_str!("../README.md")]`, so all ten fences are
doctests and `cargo test` fails the next time the docs fall behind.
Fences that would touch the filesystem, or that need a live failure to
illustrate, are `no_run`: compiled, not executed. The rest run against
an in-memory database set up in hidden lines.

Verified the wiring is load-bearing, not decorative: reverting one
example to its old `allocate_tagged(b"row-a", 42)` form fails the suite
with "expected `Tag`, found integer".

The prose the snippets sit in is corrected to match — tags are non-zero
and untagged means `tag()` returns None (the "tag 0 means untagged"
model predates I126), and the API table now names `Handle`/`Tag` rather
than `u64`/`u32`.

Both `Stats` fields were also described wrongly, in opposite
directions. `total_pages` claimed to match `Superblock.total_pages`
while being filled from the PHYSICAL page count, which is the more
useful of the two to name explicitly since they diverge exactly when a
crash leaves orphan pages in the tail. `file_size_bytes` claimed it
"may exceed `total_pages * PAGE_SIZE`" when it is computed from that
same count and so can never exceed itself.

Finally, `get_root_name` and `clear_root_name` both run the same name
validation `set_root_name` does, so `""` or a 25-byte name is an
`InvalidRootName` error, not a miss. Their `# Errors` sections said
"Only on poisoning" and "NoActiveTransaction if no transaction is
open", inviting a caller to treat any Err from a lookup as a fatal
drop-and-reopen and tear down a healthy handle. `#[warn(missing_errors_doc)]`
guarantees the section exists but not that it is complete, which is
what makes an omission like this easy to trust. Both sections now list
it, and a test pins the contract — including the half that is easy to
over-correct: a valid-but-unbound name is still Ok.

Closes #99.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Public API: README error-tier and example code contradict the implementation

1 participant